feat(sim): publish structured /rmf_simulation/collision_event JSON on emergency stop#161
feat(sim): publish structured /rmf_simulation/collision_event JSON on emergency stop#161jayDevCodes wants to merge 1 commit intoopen-rmf:mainfrom
Conversation
… emergency stop (no new msg types)
|
@jayDevCodes, I think @yenode is already working on this specific issue. I'd rather not have more than one person work on such a small task. Perhaps you can look at rmf demos and @yenode's PR and see if we can automate success criterion for the tests. |
|
Hi @arjo129, We reviewed Yenode’s work and the demos. To automate the test pass/fail result, we plan to update the slotcar plugin so it publishes a ROS2 flag when an emergency stop happens (for example: In the stress_test, we will add a subscriber that listens to this flag. The test will fail if the flag is triggered or if any robot’s mode becomes This way we can use Gazebo’s built-in collision detection through the slotcar logic and connect it to a topic that can be tested automatically. We can prepare a draft PR soon. Please let me know if this approach looks good to you. Thanks! |
| // ----------------------- collision publisher init (non-invasive) ---------- | ||
| // Create a per-model publisher and optionally read a pause parameter. | ||
| try { | ||
| std::lock_guard<std::mutex> lk(g_collision_pub_map_mutex); |
There was a problem hiding this comment.
Why do this? It makes no sense to have a per model publisher it'll just flood DDS discovery and make detecting failures harder.
There was a problem hiding this comment.
The std::lock_guardstd::mutex lk(g_collision_pub_map_mutex); protects access to the global g_collision_pub_map to avoid data races — init_ros_node() may insert the publisher while emergency_stop() (or other threads) read/publish from the map, so we must synchronize those accesses.
There was a problem hiding this comment.
Please don't upload macos metadata.
There was a problem hiding this comment.
Thanks for pointing that out. I’ve removed .DS_Store from the repository and added it to .gitignore so macOS metadata files won’t be committed again.
| { | ||
| return _lookahead_point; | ||
| } | ||
| } No newline at end of file |
| } | ||
|
|
||
| // Small local struct to hold per-model collision publisher config. | ||
| struct CollisionPubData { |
|
|
||
|
|
||
| // Escape minimal JSON characters in simple strings (no external deps). | ||
| static std::string EscapeJson(const std::string &s) { |
There was a problem hiding this comment.
We probably should avoid json magic here. Either use nlohman-json or we should just use a custom message.
|
@jayDevCodes , please update the PR template honestly as to whether you used GenAI for the PR. The OSRA policy for GenAI requires that you disclose any use of gen AI (we. are not against genAI but you need to be honest). Failure to do so will not bode well for any GSoC applicant. |
|
@arjo129 I only used an AI assistant to help spot/fix small bugs and tidy up the code for readability. The design and core logic are implemented by me, which is the main part of this work. — happy to revert or adjust any edits you prefer. |
|
Have you read the policy I linked? Any use of AI must be declared in the PR description and commits. Not declaring it means dishonesty. I will be closing this PR as there is a better solution already. |
3 participants
This small change publishes a structured collision event on when a model triggers an emergency stop.
pause_on_collisioncan be declared to request pause (not implemented in this context).How this helps:
/rmf_simulation/collision_eventand fail tests automatically when collisions occur.